-
Couldn't load subscription status.
- Fork 42
update pagination for collection-search #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
* handle collection paging differently * test next link
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
| assert len(resp.json()["collections"]) == 2 | ||
|
|
||
|
|
||
| @requires_pgstac_0_9_2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will skip the test for old version of pgstac
IMO it will be a bit bold to restrict pgstac>=0.9.2 just because of the collection-pagination issue.
We will need to add a note in the documentation stating that collection-search is partially supported for 0.8.*
|
|
||
|
|
||
| @requires_pgstac_0_9_2 | ||
| @pytest.mark.xfail(strict=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking this test to fail because it might be a pgstac bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment was marked as resolved.
This comment was marked as resolved.
…nabled (#179) * fallback to all_collections when `CollectionSearchExtension` is not enabled * test all_collection fallback
| query=param_string, | ||
| fragment=u.fragment, | ||
| ).geturl() | ||
| href = merge_params(self.url, self.prev["body"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted this and now we will return URLs with offset=0
| assert {"root", "self", "previous"} == {link["rel"] for link in links} | ||
| prev_link = list(filter(lambda link: link["rel"] == "previous", links))[0] | ||
| # offset=0 should not be in the previous link (because it's useless) | ||
| assert "offset" not in prev_link["href"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in our tests we have only 2 collections, and in pgstac the limit is set to 10 so
https://github.com/stac-utils/pgstac/blob/main/src/pgstac/sql/004a_collectionsearch.sql#L106
will not be True,this is why we don't have offset=0 previous link
cc @bitner
|
This PR is ready for review 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting this over the finish line @vincentsarago!
Co-authored-by: Henry Rodman <[email protected]>
this PR adds a tests showing that the recent collection-search addition breaks the pagination code
cc @hrodmn